Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/core/src/hive/plan.rs`:
- Around line 233-247: The plan builder sets greedy_evaluate: true
unconditionally for Goal::Build, forcing Evaluate steps to run even when
Evaluate { cached_evaluation: Some(..) } can satisfy the step; update the
Goal::Build branch in NodePlan construction to set greedy_evaluate to false
whenever the initial Evaluate has cached_evaluation.is_some() (i.e., inspect the
cached_evaluation used when creating Step::Evaluate in that branch) so the
planner will respect the cache fast-path and skip running evaluate_task; modify
the NodePlan creation logic that constructs Context, steps
(Step::Evaluate(Evaluate { cached_evaluation })), and the greedy_evaluate field
to conditionally assign greedy_evaluate = cached_evaluation.is_none() (or
equivalent) for Goal::Build.

---

Nitpick comments:
In `@crates/cli/src/apply.rs`:
- Around line 128-134: The match arm creating cached_evaluations unnecessarily
calls cache.clone() then dereferences it; replace the deref of the temporary Arc
with a direct deref of the existing Arc (use *cache rather than *cache.clone())
in the if-let pattern that checks Some(ref cache) and the HiveLocation::Flake
branch so the code reads if let Some(ref cache) = *cache && let
HiveLocation::Flake { ref prefetch, .. } = *location { ... } ensuring you only
borrow the inner value instead of cloning the Arc.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/cli/src/apply.rs`:
- Around line 178-187: The tokio::spawn call in apply() currently
fire-and-forgets the cache persistence task (the block that calls
cache.store_evaluation(prefetch, &cache_name, evaluated_path)), which can let
apply() return before writes complete; instead, collect the JoinHandle returned
by tokio::spawn (for the task that calls cache.store_evaluation) into a
Vec<JoinHandle<()>> (or similar) inside apply(), and before returning from
apply() await/join all handles (e.g., via futures::future::join_all or awaiting
each handle) to ensure store_evaluation completes; update the code around
tokio::spawn, the async move closure, and apply()'s return path so the handles
are awaited and any JoinError is handled/logged appropriately.

In `@crates/core/src/cache/mod.rs`:
- Around line 253-255: The SQL query in crates::core::cache module that filters
on evaluation_cache.node_name using json_each($4) currently has a hard-coded
"limit 100" which can drop valid cache hits; remove the fixed "limit 100" (or
replace it with a configurable parameter passed into the query) so all matching
nodes from json_each($4) are returned; locate the SQL string that references
evaluation_cache.node_name and json_each($4) and either delete the "limit 100"
clause or wire a new limit parameter and use that in the query execution call so
large node selections are not truncated.
- Around line 497-505: The deletion in gc_evaluation_cache currently discards
errors by assigning the execute result to `_`; change this so failures are
handled: capture the result of the sqlx::query!(...).execute(&self.pool).await,
and either propagate the error from gc_evaluation_cache (return a Result and use
`?`) or log the error (e.g., via tracing::error/process logger) with context
including the path identifiers (flake_path_digest, flake_path_name,
output_path_digest, output_path_name) so DB cleanup failures are visible; update
the gc_evaluation_cache function signature to return Result if propagating, or
keep the signature and log the error when execute returns Err.
- Around line 428-433: The GC query currently fetches a single unordered LIMIT
50 slice (the EvaluationPaths query that assigns evaluation_paths), so shutdown
GC can miss stale rows in larger caches; change it to a deterministic, batched
sweep: add an explicit ORDER BY on a stable key (e.g., primary key or timestamp)
and iterate in a loop fetching batches (LIMIT N with a cursor/last_seen key or
OFFSET) until no more rows, performing deletion per-batch, so every row is
eventually considered; update the code around evaluation_paths / the query_as!
call and the surrounding shutdown GC routine to use the ordered, paged
fetch-and-delete loop instead of the single fetch_all.

In `@crates/nix_client/src/lib.rs`:
- Around line 754-760: The NixDaemonClient protocol error is reporting the wrong
wanted version and operation: in the block checking self.writer.version() <
ProtocolVersion::from_parts(1, 22) update the
NixDaemonClientError::NixDaemonProtocolVersion construction to use wanted:
ProtocolVersion::from_parts(1, 22) and change operation: "QueryMissing".into()
to operation: "QueryDerivationOutputMap".into() so the error matches the actual
version check and operation name.
- Around line 381-389: The handlers for STDERR_READ and STDERR_WRITE currently
call unimplemented!() which will panic if the daemon sends those messages;
replace the panics with a graceful error return or no-op handling: for
STDERR_READ call self.read_value().await? to obtain the payload length and then
return an Err with a clear error (or Ok(()) if you choose to ignore stderr), and
for STDERR_WRITE either read the incoming bytes (using existing read logic) and
discard them or return a descriptive error; update the match arm for STDERR_READ
and STDERR_WRITE in the same block so they propagate Result errors rather than
panicking, referencing STDERR_READ, STDERR_WRITE, and read_value().

---

Nitpick comments:
In `@crates/core/src/hive/node.rs`:
- Around line 346-395: Add a unit test that exercises Target::create_ssh_args
with force_quiet set to true to ensure "-q" is appended and any "-v"/verbosity
flags are suppressed; construct a Target (e.g.,
Target::from_host("hello-world")), pass a SubCommandModifiers with ssh_verbosity
> 0, call target.create_ssh_args(subcommand_modifiers, true).unwrap(), then
assert the returned Vec<String> contains "-q" and contains no entries starting
with "-v" (and also assert behavior is consistent with create_ssh_opts if
desired).